-
-
Notifications
You must be signed in to change notification settings - Fork 278
feat: filter search for commit change type #1381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat: filter search for commit change type #1381
Conversation
4dd1521
to
5c52c3d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1381 +/- ##
==========================================
+ Coverage 97.33% 97.55% +0.21%
==========================================
Files 42 57 +15
Lines 2104 2656 +552
==========================================
+ Hits 2048 2591 +543
- Misses 56 65 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
"use_search_filter": True, | ||
"use_jk_keys": False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know whether we should set it as the default, probably not for v4. I would suggest making it configurable. Also, this now only works for conventional_commits
, we would like it to work for all cz rules. Following up with this discussion, we probably could make the whole questionary behavior configurable, but that could be another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review! I will modify the search function to work for all cz rules. As for making it configurable, we can open another issue for that. Does that sound good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably say we should make it configurable first to retain the current behavior and let those who want to play with it opt in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Lee-W,
I have tested the search filter functionality using the configuration approach:
[[tool.commitizen.customize.questions]]
type = "list"
name = "change_type"
message = "Select the type of change you are committing"
use_search_filter = true
use_jk_keys = false
It works as expected, so I've:
- Removed the modifications to conventional_commits.py
- Added documentation for the feature in customization.md
- Added test cases to verify the functionality
The search filter can now be enabled through configuration without modifying the core code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but if we do it this way, doesn't that mean we won't be able to use it in other cz rules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to ask if cz_empty
and cz_no_default
imply that JIRA and conventional commits can also be customized? If that's the case, should we address this in the current issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JIRA and conventional commits can also be customized?
These two are somewhat customizable, but just not at the same level as cz_customize
.
e.g., they have the fixed set of questions that cannot yet be customized at this moment but some of the behavior can still be changed by our config file
We don't need to do cz_empty
in this PR. but we might want to pass these 2 options into all cz rules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is what you meant,
But I added a question_type field in Settings, where users can choose "select" or "list".
This solution works based on my testing and fulfills the spec.
By the way, I want to ask what would the jira rule be like?
jira rule does not have list now.
.cz.toml
[tool.commitizen]
name = "cz_conventional_commits"
tag_format = "$version"
version_scheme = "semver"
version = "0.0.1"
update_changelog_on_bump = true
major_version_zero = true
question_type = "select"
conventional_commits.py
def __init__(self, config: BaseConfig):
super().__init__(config)
self.question_type = self.config.settings.get("question_type", "list")
def questions(self) -> Questions:
# Default questions
questions: Questions = [
{
"type": self.question_type,
"name": "prefix",
"message": "Select the type of change you are committing",
...
if questions[0]["type"] == "select":
questions[0].update({"use_search_filter": True, "use_jk_keys": False})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait...
I thought we only need use_search_filter
and use_jk_keys
to config this kind of search on questions that is select or list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thought is that the user only needs to set the type.
Since select type requires additional options like:
"use_search_filter": true,
"use_jk_keys": false
but list does not,
I apply these settings in code when the type is select,
so the user doesn't need to configure them manually.
Do you think config these two is a better way ?
5c52c3d
to
2f233d8
Compare
Hi @Lee-W, I have tested the search filter functionality using the configuration approach:
It works as expected, so I've:
|
Hey @Yusin0903 , just noticed the CI is failing. Could you please take a look? Thanks 🙂 |
2f233d8
to
c0b1e05
Compare
We could also deprecate |
Yep, this is a fantastic idea! |
2489e23
to
c0b1e05
Compare
Description
Added search filter functionality to commit type selection in Conventional Commits. This allows users to quickly find commit types by typing part of the type name or description, enhancing user experience especially when working with many commit types.
Changes Made
Checklist
poetry all
locally to ensure this change passes linter check and testExpected behavior
When a user types characters during commit type selection, the list of options is filtered to show only those containing the typed characters. For example, typing "fix" will show the item that contains "fix" .
Steps to Test This Pull Request
Additional context